Looking good so far! Dictionaries can be useful for building up keyword arguments / parameters with values that have to be overwritten, since it's easy to overwrite entries in a dictionary.

Codecov Report

Attention: Patch coverage is 0% with 16 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (dd9148a) to head (9cd5f7e).

@@          Coverage Diff          @@
##            main    #123   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files         31      31           
  Lines       1729    1742   +13     
=====================================
- Misses      1729    1742   +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@glwagner in the most recent commit I added a way to merge the user supplied boundary_conditions while keeping the hard coded T and S boundary conditions that aren't supplied by the user (ie we keep the surface fluxes of T, S even if we need to use user-supplied T or S conditions at the bottom or on immersed boundaries). It seems to work, but its ugly... any suggestions to do this more cleanly?

@glwagner in the most recent commit I added a way to merge the user supplied boundary_conditions while keeping the hard coded T and S boundary conditions that aren't supplied by the user (ie we keep the surface fluxes of T, S even if we need to use user-supplied T or S conditions at the bottom or on immersed boundaries). It seems to work, but its ugly... any suggestions to do this more cleanly?

I think what you wrote captures the gist of it! But maybe we can make a few tweaks, I'll suggest something

Hmm yes actually I realized we need to do something more like this:

using Oceanaingans.BoundaryConditions: DefaultBoundaryCondition

# create a utility that filters `DefaultBoundaryCondition`
boundaries = (:west, :east, :south, :north, :bottom, :top, :immersed)

"""
    field_bcs_to_dict(field_bcs)

Return a `Dict` with the properties of `field_bcs` filtered to remove `DefaultBoundaryCondition`.
"""
function field_bcs_to_dict(field_bcs)
    all_bcs = Dict(boundary => getproperty(field_bcs, boundary) for boundary in boundaries)
    non_default_bcs = filter(kv -> !isa(kv[2], DefaultBoundaryCondition), all_bcs)
    return non_default_bcs
end

# inside ocean_simulation...

# make clear these are the defaults
default_boundary_conditions = Dict(
    :u => Dict(:top => FluxBoundaryCondition(Jᵘ), :bottom => u_bot_bc),
    :v => Dict(:top => FluxBoundaryCondition(Jᵛ), :bottom => v_bot_bc),
    :T => Dict(:top => FluxBoundaryCondition(Jᵀ)),
    :S => Dict(:top => FluxBoundaryCondition(Jˢ))
)

# Build a list of the default + user-specified boundary condition names
user_bc_names = keys(boundary_conditions)
bc_names = tuple(:u, :v, :T, :S, user_bc_names...) |> unique

# Merge user and default boundary conditions
merged_boundary_conditions = Dict()

for fieldname in bc_names

    default_bcs = get(default_boundary_conditions, fieldname, Dict())

    if fieldname ∈ user_bc_names
        user_bcs = field_bcs_to_dict(boundary_conditions[fieldname])
    else
        user_bcs = Dict()
    end

    # Generate a dictionary specifying `FieldBoundaryConditions` for each `fieldname`
    bc_dict = merge(default_bcs, user_bcs)
    merged_boundary_conditions[fieldname] = FieldBoundaryConditions(; bc_dict...)
end

# Convert to NamedTuple
merged_boundary_conditions = NamedTuple(merged_boundary_conditions)

A few things we could do (in a future PR) are:

1. Print some information if the user overwrites the default boundary conditions
2. Make sure boundary_conditions is a NamedTuple and also populated correctly with FieldBoundaryConditions

Not super pretty either...

Realized my code sketch was missing the conversion to FieldBoundaryConditions... could be missing some other things too...

Realized my code sketch was missing the conversion to FieldBoundaryConditions... could be missing some other things too...

Ok no worries. I'll go through and test it and make sure it's all working, thanks.

I dropped the ball on this but picking it back up now and going to test it and hopefully it'll be ready for review this week!